Skip to content

collapse sidebar on home page to keep focus on content, other pages have it open #531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

collapse sidebar on home page to keep focus on content, other pages have it open #531

wants to merge 3 commits into from

Conversation

@evantahler
Copy link
Contributor

@evantahler evantahler commented Oct 23, 2025

I'm emulating clicks to trigger the animations.

@evantahler evantahler requested a review from sdserranog October 23, 2025 18:26
@vercel
Copy link

vercel bot commented Oct 23, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
docs Error Error Oct 23, 2025 6:48pm

@evantahler evantahler requested a review from vfanelle October 23, 2025 18:26
evantahler
evantahler commented Oct 23, 2025
View reviewed changes
app/_layouts/shared-layout.tsx
Comment on lines +84 to +87
// Get the current pathname to determine initial sidebar state
const headersList = await headers();
const pathname = headersList.get("x-pathname") || "";
const isHomePage = pathname.endsWith("/home");
Copy link
Contributor Author

@evantahler evantahler Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need check on ssr if we are on the homepage or not to start with the sidebar collapsed or not, otherwise you see a jank animation post-pageload.

@evantahler evantahler marked this pull request as ready for review October 23, 2025 18:28
@evantahler
Copy link
Contributor Author

evantahler commented Oct 23, 2025

@sdserranog any idea why the build might be failing here?

[Parsing files]
Did not find a data-pagefind-body element on the site.
↳ Indexing all <body> elements on the site.
1 page found without an <html> element.
Pages without an outer <html> element will not be processed by default.
If adding this element is not possible, use the root selector config to target a different root element.
  * "/_not-found.html" has no <html> element

[Reading languages]
Discovered 0 languages:

[Building search indexes]
Total:
  Indexed 0 languages
  Indexed 0 pages
  Indexed 0 words
  Indexed 0 filters
  Indexed 0 sorts
Error: Pagefind was not able to build an index.
Most likely, the directory passed to Pagefind was empty or did not contain any html files.
Error: Pagefind was not able to build an index.
Most likely, the directory passed to Pagefind was empty or did not contain any html files.
 ELIFECYCLE  Command failed with exit code 1.

@sdserranog
Copy link
Contributor

sdserranog commented Oct 31, 2025

To be honest, I am not a fan of this solution. We could make some improvements, like creating a custom hook instead of a component and importing it only on the home page, so we don't need to track the pathname in each component and page. However, I don't think it's worth it, the sidebar doesn't look bad on this page, and I believe the main feature was collapsing the sidebar and opening it on hover. We can discuss it, or not, and I'm ok with the changes if we feel strongly about this.

@evantahler
Copy link
Contributor Author

evantahler commented Oct 31, 2025

I defer to @vfanelle who was the one who wanted the sidebar hidden. I'll close this PR either way, because with the layout changes this would have to change dramatically

@evantahler evantahler closed this Oct 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sdserranog sdserranog Awaiting requested review from sdserranog

@vfanelle vfanelle Awaiting requested review from vfanelle

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@evantahler @sdserranog